Skip to content

Add transactional traits for SPI and I2C#178

Closed
ryankurte wants to merge 15 commits into
rust-embedded:masterfrom
ryankurte:feature/spi-i2c-transactions
Closed

Add transactional traits for SPI and I2C#178
ryankurte wants to merge 15 commits into
rust-embedded:masterfrom
ryankurte:feature/spi-i2c-transactions

Conversation

@ryankurte

@ryankurte ryankurte commented Jan 9, 2020

Copy link
Copy Markdown
Contributor

@rust-highfive

Copy link
Copy Markdown

r? @thejpster

(rust_highfive has picked a reviewer for you, use r? to override)

Comment thread src/blocking/spi.rs Outdated
Comment thread src/blocking/spi.rs
Comment thread Cargo.toml Outdated
Comment thread src/blocking/i2c.rs Outdated
Comment thread src/blocking/i2c.rs Outdated
@ryankurte ryankurte changed the title [WIP] add transactional traits for SPI and I2C Add transactional traits for SPI and I2C Jan 21, 2020
Comment thread src/blocking/spi.rs
Comment thread src/blocking/spi.rs Outdated
Comment thread src/blocking/spi.rs Outdated
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Mar 9, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Mar 9, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Mar 9, 2020
@ryankurte

Copy link
Copy Markdown
Contributor Author

Closing out in favour of #191 for now

@ryankurte ryankurte closed this Mar 9, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Jun 2, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Jun 2, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Jun 2, 2020
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Jun 2, 2020
@eldruin eldruin mentioned this pull request Jun 17, 2020
1 task
ryankurte added a commit to ryankurte/embedded-hal that referenced this pull request Jun 24, 2020
bors Bot added a commit that referenced this pull request Oct 28, 2020
191: Added transactional SPI interface r=therealprof a=ryankurte

This PR adds a transactional interface for SPI devices (#94), compatible with linux spidev.

Split from #178 as I believe this is complete and useful, but that there is more experimentation required before (if?) the I2C component is landed, check there for previous reviews / discussion.

**Demonstrated in:**
- Linux embedded hal: rust-embedded/linux-embedded-hal#35
- STM32F4xx-hal: stm32-rs/stm32f4xx-hal#167
- embedded-spi driver abstraction (previously provided a polyfill for equivalent transactional functionality) https://github.com/ryankurte/rust-embedded-spi/pull/4/files#diff-74eea42f4e5e15399ac9184c8f2727a9R344
- sx128x radio driver: rust-iot/rust-radio-sx128x#5


**Notes:**
- `Operation::Transfer` uses one buffer to allow polyfill using the existing `Transfer` trait (with the convenient side effect of reducing memory requirements)
- `W` has a static bound as it _should_ only ever be a type with static lifetime (u8, u16 etc., not a reference), and to differentiate this from `'a` which is the lifetime of the data in the object and only bound to the function
- `exec(.., &mut [Operation])` is chosen over `exec<O: AsMut<[Operation]>(..)` as the latter imposes limits on generic types using this trait (which i ran into, see [E0038](https://doc.rust-lang.org/error-index.html#E0038))

cc. @rust-embedded/hal folks, @eldruin, @RandomInsano, @Rahix, @austinglaser for opinions / review

Co-authored-by: Ryan Kurte <ryankurte@gmail.com>
Co-authored-by: ryan <ryan@kurte.nz>
bors Bot added a commit that referenced this pull request Oct 30, 2020
223: Add transactional I2C interface r=therealprof a=eldruin

An example where a transactional I2C interface would be an improvement is when facing the problem of sending an array of data where the first item is the destination address.
At the moment this requires copying the data into a bigger array and assigning the first item to the address. With a transactional I2C two `write` operations could be chained into one transaction so that it is possible to send the address and data without an extra copy.
This is specially problematic if the data length is unknown e.g. because it comes from the user.

You can see the problem [here in the eeprom24x driver](https://github.com/eldruin/eeprom24x-rs/blob/f75770c6fc6dd8d591e3695908d5ef4ce8642566/src/eeprom24x.rs#L220). In this case I am lucky enough to have the page upper limit so that the copy workaround is possible.

With this PR a bunch of code and macros could be replaced by doing something similar to this:
```rust
let user_data = [0x12, 0x34, ...]; // defined somewhere else. length may be unknown.
let target_address = 0xAB;
let mut ops = [
  Operation::Write(&[target_address]),
  Operation::Write(&user_data),
];
i2cdev.try_exec(DEV_ADDR, &ops)
```

I added a PoC [here in linux-embedded-hal](https://github.com/eldruin/linux-embedded-hal/blob/7512dbcc09faa58344a5058baab8826a0e0d0160/src/lib.rs#L211) including [an example](https://github.com/eldruin/linux-embedded-hal/blob/transactional-i2c/examples/transactional-i2c.rs) of a driver where a classical combined write/read is performed through the transactional interface.

Note: A default implementation of the `Transactional` trait like in #191 is not possible because STOPs would always be sent after each operation. What is possible is to do is the other way around. This includes an implementation of the `Write`, `Read` and `WriteRead` traits for `Transactional` implementers.

This is based on previous work from #178 by @ryankurte and it is similar to #191 

TODO:
- [x] Add changelog entry

Co-authored-by: Diego Barrios Romero <eldruin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Review is incomplete T-hal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants